Pass in strong password to OSD integ test if version >= 2.12.0 #4334

derek-ho · 2024-01-10T01:25:44Z

Description

OSD health check for integ test is using admin:admin, this changes it to admin:myStrongPassword123! if version >= 2.12.0

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Derek Ho <[email protected]>

… into osd-integ

derek-ho · 2024-01-10T01:28:02Z

src/test_workflow/integ_test/service_opensearch_dashboards.py

@@ -38,6 +38,7 @@ def __init__(
        super().__init__(work_dir, version, distribution, security_enabled, additional_config, dependency_installer)
        self.dist = Distributions.get_distribution("opensearch-dashboards", distribution, version, work_dir)
        self.install_dir = self.dist.install_dir
+        self.password = 'myStrongPassword123!' if float('.'.join(self.version.split('.')[:2])) >= 2.12 else 'admin'


This PR should address this failure: https://build.ci.opensearch.org/blue/organizations/jenkins/integ-test-opensearch-dashboards/detail/integ-test-opensearch-dashboards/5025/pipeline, however I am not sure how the OS binary is pulled - is it given myStrongPassword123! as the password somehow?

Hi @derek-ho , can you try semver package instead of float check? See #4304

@gaiksaya done. Question still stands though, I am confused about the overall setup of: https://build.ci.opensearch.org/blue/organizations/jenkins/integ-test-opensearch-dashboards/detail/integ-test-opensearch-dashboards/5025/pipeline/, would this change be sufficient to pass the integ test? Would the backend be spun up with the strong password as expected if version >= 2.12.0?

The integration test set up can be found here: https://github.com/opensearch-project/opensearch-build/blob/main/src/test_workflow/integ_test/ for both OS and OSD.
Maybe try to parse this directory to check for anything admin:admin?
If it feels there is a lot of if/else going on each module, maybe extract the logic into a common library (utils?) (something like get_password(version)) that can be used universally. Example being this system module.

WDYT @prudhvigodithi ?

codecov · 2024-01-10T01:29:35Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6eb6f8b) 91.34% compared to head (6bd68e9) 91.35%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4334   +/-   ##
=======================================
  Coverage   91.34%   91.35%           
=======================================
  Files         189      190    +1     
  Lines        6169     6175    +6     
=======================================
+ Hits         5635     5641    +6     
  Misses        534      534

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Derek Ho <[email protected]>

derek-ho · 2024-01-10T15:36:56Z

src/test_workflow/integ_test/distribution_tar.py

@@ -33,7 +33,7 @@ def install(self, bundle_name: str) -> None:
    @property
    def start_cmd(self) -> str:
        start_cmd_map = {
-            "opensearch": "export OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! && ./opensearch-tar-install.sh",
+            "opensearch": "env OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! ./opensearch-tar-install.sh",


Changed this to env from suggestion from @peterzhuamazon. Can somebody with insight explain why this might work better? From the jenkins failures it looks like either the cluster is not being spun up successfully with strong password, or the health check is not using the correct credential, even though I have made changes that I think should solve both of these. It seems like this command is run with python subprocess, might that have something to do with it?

tests/tests_test_workflow/test_integ_workflow/integ_test/test_service_opensearch.py

src/test_workflow/integ_test/utils.py

Signed-off-by: Derek Ho <[email protected]>

prudhvigodithi · 2024-01-10T16:52:47Z

@derek-ho I see the error from OSD as 401: {"statusCode":401,"error":"Unauthorized","message":"Authentication Exception"}, please confirm if this the exact error? https://build.ci.opensearch.org/blue/organizations/jenkins/integ-test-opensearch-dashboards/detail/integ-test-opensearch-dashboards/5025/pipeline

derek-ho · 2024-01-10T18:57:49Z

@derek-ho I see the error from OSD as 401: {"statusCode":401,"error":"Unauthorized","message":"Authentication Exception"}, please confirm if this the exact error? https://build.ci.opensearch.org/blue/organizations/jenkins/integ-test-opensearch-dashboards/detail/integ-test-opensearch-dashboards/5025/pipeline

@prudhvigodithi Yes I believe so. I see logs from that run: https://ci.opensearch.org/ci/dbc/integ-test-opensearch-dashboards/3.0.0/7076/linux/arm64/tar/test-results/5025/integ-test/reportsDashboards/with-security/local-cluster-logs/id-1/stdout.txt, indicating that the admin password was successfully set to a strong password. Then authentication fails for user. Thus I expect that the only change needed to is to change the password the user uses in integ test.

…rd succcesfully set Signed-off-by: Derek Ho <[email protected]>

gaiksaya · 2024-01-10T19:58:15Z

@derek-ho This might be the culprit: https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/integtest.sh

gaiksaya

LGTM! Needs changes in https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/integtest.sh

Also @derek-ho Do we need to parse all the files again to call this method or have you taken care of it already? https://github.com/search?q=repo%3Aopensearch-project%2Fopensearch-build%20myStrongPassword123&type=code

Thanks!

prudhvigodithi · 2024-01-10T20:45:24Z

LGTM! Needs changes in https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/integtest.sh

Looks like this is already updated right @gaiksaya ?

prudhvigodithi · 2024-01-10T20:48:05Z

Also @derek-ho dont want to block this, but please open a new PR for this change mentioned by Sayali.

Also @derek-ho Do we need to parse all the files again to call this method or have you taken care of it already? https://github.com/search?q=repo%3Aopensearch-project%2Fopensearch-build%20myStrongPassword123&type=code

To do this move the utils.py to to a common file under https://github.com/opensearch-project/opensearch-build/tree/main/src and use it for other workflows as well.

Thank you

gaiksaya · 2024-01-10T20:50:36Z

LGTM! Needs changes in https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/integtest.sh

Looks like this is already updated right @gaiksaya ?

Not in 2.x branch https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/2.x/integtest.sh#L83

derek-ho · 2024-01-10T20:53:25Z

LGTM! Needs changes in https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/integtest.sh

Looks like this is already updated right @gaiksaya ?

Not in 2.x branch https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/2.x/integtest.sh#L83

This backport PR is open: opensearch-project/opensearch-dashboards-functional-test#992, let me nudge maintainers now.

prudhvigodithi · 2024-01-10T21:02:45Z

True not for 2.x, but for 3.0.0 since its already on main, I just triggered a new build https://build.ci.opensearch.org/job/integ-test-opensearch-dashboards/5034/console and dont see 401 errors anymore.

derek-ho added 17 commits January 9, 2024 09:38

Change password based on version in integtest health check

e1ad15a

Signed-off-by: Derek Ho <[email protected]>

Remove hard coded 3

46cd35b

Signed-off-by: Derek Ho <[email protected]>

Add version check same as benchmark

a7ba5e4

Signed-off-by: Derek Ho <[email protected]>

Fix to minimize changes

3097bbc

Signed-off-by: Derek Ho <[email protected]>

Revert logging line

f493f24

Signed-off-by: Derek Ho <[email protected]>

Remove non-existent args param

5d7f060

Signed-off-by: Derek Ho <[email protected]>

try adding a test to increase codecov

9ae642e

Signed-off-by: Derek Ho <[email protected]>

Add test

574e40a

Signed-off-by: Derek Ho <[email protected]>

Fix test

034f0cb

Signed-off-by: Derek Ho <[email protected]>

fix capital

4aac0c2

Signed-off-by: Derek Ho <[email protected]>

remove extra line

d6fa3d5

Signed-off-by: Derek Ho <[email protected]>

Fix test

3185ebe

Signed-off-by: Derek Ho <[email protected]>

Fix test

2d77952

Signed-off-by: Derek Ho <[email protected]>

Fix test

42fccb2

Signed-off-by: Derek Ho <[email protected]>

Fix test

d76d9b2

Signed-off-by: Derek Ho <[email protected]>

Fix version to hit more lines

c2b1374

Signed-off-by: Derek Ho <[email protected]>

Update password based on version for OSD

48fbe24

Signed-off-by: Derek Ho <[email protected]>

derek-ho requested review from dblock, peterzhuamazon, bbarani, gaiksaya, rishabh6788, zelinh, jordarlu, prudhvigodithi, Divyaasm and tianleh as code owners January 10, 2024 01:25

github-actions bot added the repeat-contributor label Jan 10, 2024

Merge branch 'main' of github.com:opensearch-project/opensearch-build…

8c86ce6

… into osd-integ

derek-ho commented Jan 10, 2024

View reviewed changes

derek-ho added 5 commits January 9, 2024 21:07

change to semver impl

e450957

Signed-off-by: Derek Ho <[email protected]>

Reorg into utils and test and change export to env

0dcfd3d

Signed-off-by: Derek Ho <[email protected]>

hello

207be45

Signed-off-by: Derek Ho <[email protected]>

Fix import

ec6a0a0

Signed-off-by: Derek Ho <[email protected]>

fix test assertion

20be6ff

Signed-off-by: Derek Ho <[email protected]>

derek-ho commented Jan 10, 2024

View reviewed changes

DarshitChanpura reviewed Jan 10, 2024

View reviewed changes

tests/tests_test_workflow/test_integ_workflow/integ_test/test_service_opensearch.py Outdated Show resolved Hide resolved

src/test_workflow/integ_test/utils.py Show resolved Hide resolved

Fix assertion

5d8d699

Signed-off-by: Derek Ho <[email protected]>

revert changes to export after checking logs to see that admin passwo…

6bd68e9

…rd succcesfully set Signed-off-by: Derek Ho <[email protected]>

gaiksaya approved these changes Jan 10, 2024

View reviewed changes

prudhvigodithi merged commit eb1d8fe into opensearch-project:main Jan 10, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass in strong password to OSD integ test if version >= 2.12.0 #4334

Pass in strong password to OSD integ test if version >= 2.12.0 #4334

derek-ho commented Jan 10, 2024

derek-ho Jan 10, 2024

gaiksaya Jan 10, 2024

derek-ho Jan 10, 2024

gaiksaya Jan 10, 2024 •

edited

Loading

codecov bot commented Jan 10, 2024 •

edited

Loading

derek-ho Jan 10, 2024

prudhvigodithi commented Jan 10, 2024 •

edited

Loading

derek-ho commented Jan 10, 2024 •

edited

Loading

gaiksaya commented Jan 10, 2024

gaiksaya left a comment

prudhvigodithi commented Jan 10, 2024

prudhvigodithi commented Jan 10, 2024

gaiksaya commented Jan 10, 2024

derek-ho commented Jan 10, 2024

prudhvigodithi commented Jan 10, 2024

Pass in strong password to OSD integ test if version >= 2.12.0 #4334

Pass in strong password to OSD integ test if version >= 2.12.0 #4334

Conversation

derek-ho commented Jan 10, 2024

Description

Issues Resolved

derek-ho Jan 10, 2024

Choose a reason for hiding this comment

gaiksaya Jan 10, 2024

Choose a reason for hiding this comment

derek-ho Jan 10, 2024

Choose a reason for hiding this comment

gaiksaya Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Jan 10, 2024 • edited Loading

Codecov Report

derek-ho Jan 10, 2024

Choose a reason for hiding this comment

prudhvigodithi commented Jan 10, 2024 • edited Loading

derek-ho commented Jan 10, 2024 • edited Loading

gaiksaya commented Jan 10, 2024

gaiksaya left a comment

Choose a reason for hiding this comment

prudhvigodithi commented Jan 10, 2024

prudhvigodithi commented Jan 10, 2024

gaiksaya commented Jan 10, 2024

derek-ho commented Jan 10, 2024

prudhvigodithi commented Jan 10, 2024

gaiksaya Jan 10, 2024 •

edited

Loading

codecov bot commented Jan 10, 2024 •

edited

Loading

prudhvigodithi commented Jan 10, 2024 •

edited

Loading

derek-ho commented Jan 10, 2024 •

edited

Loading